-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
V2: Update query keys for partial matching #441
V2: Update query keys for partial matching #441
Conversation
01934ec
to
519e4ad
Compare
Signed-off-by: Timo Stamm <ts@timostamm.de>
519e4ad
to
d4b1fd2
Compare
Signed-off-by: Timo Stamm <ts@timostamm.de>
Signed-off-by: Timo Stamm <ts@timostamm.de>
Signed-off-by: Timo Stamm <ts@timostamm.de>
Signed-off-by: Timo Stamm <ts@timostamm.de>
function messageKey( | ||
message: ReflectMessage, | ||
pageParamKey?: string, | ||
): Record<string, unknown> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that the "page param" is not typically part of the key for infinite queries. Seems like a good fit to support it here. It means that the users doesn't have to omit the page param field with createConnectQueryKey
.
const customTransport = mockPaginatedTransport({ | ||
items: ["Intercepted!"], | ||
page: 0n, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression needs to move up. Otherwise, a new transport is created every time the hook is rendered, and the key changes.
As noted in #418 (comment), this can be a footgun. I think it should be sufficient to call it out in the documentation, and to give an example. WDYT, @paul-sachs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Once we've taken transport into account as a key, it's important that it gets memoized
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup!
): ConnectInfiniteQueryKey { | ||
return [...createConnectQueryKey(schema, input), "infinite"]; | ||
const props: ConnectQueryKey[1] = | ||
params.schema.kind == "rpc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, interesting. I guess this allows for invalidating a whole service if necessary.
This changes the type for Connect query keys to be:
The function
createConnectQueryKey
is updated to accept key parameters as an options object:The transport is now part of the key to support multiple transport without tainting the cache (#418).
All key parameters except
schema
are optional, which allows to create partial keys (#375) for filtering.The updated function replaces
createConnectInfiniteQueryKey
. To create a key for an infinite query, usecardinality: "infinite"
and thepageParamKey
option.